Skip to content

Obtiene SHA correcto para calcular entradas faltantes#2751

Merged
rtobar merged 1 commit into3.12from
arregla-pr-comment
Nov 20, 2023
Merged

Obtiene SHA correcto para calcular entradas faltantes#2751
rtobar merged 1 commit into3.12from
arregla-pr-comment

Conversation

@rtobar
Copy link
Copy Markdown
Collaborator

@rtobar rtobar commented Nov 20, 2023

La versión anterior usaba github.event.pull_request.merge_commit_sha para obtener el commit con el merge entre la rama origen y la rama destino. Sin embargo, este atributo no siempre está disponible. En primer lugar, el atributo puede ser un string o null:

https://docs.github.com/en/webhooks/webhook-events-and-payloads?actionType=opened#pull_request

Y segundo, la documentación aclara que el atributo se establece sólo si el atributo "mergeable" es "true", lo cual no ocurre necesariamente de forma inmediate después de abrir un PR. Según
https://docs.github.com/en/free-pro-team@latest/rest/pulls/pulls?apiVersion=2022-11-28#get-a-pull-request:

"""
The value of the mergeable attribute can be true, false, or null. If the value is null, then GitHub has started a background job to compute the mergeability. After giving the job time to complete, resubmit the request. When the job finishes, you will see a non-null value for the mergeable attribute in the response. If mergeable is true, then merge_commit_sha will be the SHA of the test merge commit. """

Este delay en el cálculo de la mergeabilidad de los PRs es probablemente lo que está causando que nuestra acción no funcione como deseamos: al ejecutarse la primera vez cuando se abre el PR, la acción NO tiene un merge_commit_sha establecido, por lo que la acción "checkout" procede con el valor por defecto para el evento pull_request_target, que es la rama destino, por lo que todas las entradas aparecen como no traducidas.

Este commit cambia el SHA al que hacemos checkout al principio de esta acción para simplemente obtener el HEAD de la rama de origen. En retrospectiva, obtener el commit de merge no reporta beneficios.

La versión anterior usaba github.event.pull_request.merge_commit_sha
para obtener el commit con el merge entre la rama origen y la rama
destino. Sin embargo, este atributo no siempre está disponible. En
primer lugar, el atributo puede ser un string o null:

https://docs.github.com/en/webhooks/webhook-events-and-payloads?actionType=opened#pull_request

Y segundo, la documentación aclara que el atributo se establece sólo si
el atributo "mergeable" es "true", lo cual no ocurre necesariamente de
forma inmediate después de abrir un PR. Según
https://docs.github.com/en/free-pro-team@latest/rest/pulls/pulls?apiVersion=2022-11-28#get-a-pull-request:

"""
The value of the mergeable attribute can be true, false, or null. If the
value is null, then GitHub has started a background job to compute the
mergeability. After giving the job time to complete, resubmit the
request. When the job finishes, you will see a non-null value for the
mergeable attribute in the response. If mergeable is true, then
merge_commit_sha will be the SHA of the test merge commit.
"""

Este delay en el cálculo de la mergeabilidad de los PRs es probablemente
lo que está causando que nuestra acción no funcione como deseamos: al
ejecutarse la primera vez cuando se abre el PR, la acción NO tiene un
merge_commit_sha establecido, por lo que la acción "checkout" procede
con el valor por defecto para el evento pull_request_target, que es la
rama destino, por lo que todas las entradas aparecen como no traducidas.

Este commit cambia el SHA al que hacemos checkout al principio de esta
acción para simplemente obtener el HEAD de la rama de origen. En
retrospectiva, obtener el commit de merge no reporta beneficios.

Signed-off-by: Rodrigo Tobar <rtobar@icrar.org>
@rtobar rtobar requested review from cmaureir and mmmarcos November 20, 2023 16:22
Copy link
Copy Markdown
Collaborator

@mmmarcos mmmarcos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@Sublian
Copy link
Copy Markdown
Contributor

Sublian commented Nov 20, 2023

Excellent 👌

@rtobar rtobar merged commit ff774a0 into 3.12 Nov 20, 2023
@rtobar rtobar deleted the arregla-pr-comment branch November 20, 2023 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants